Remove pymongo dependency from this project by squeaky-pl · Pull Request #771 · closeio/sync-engine

squeaky-pl · 2024-05-29T18:56:31Z

The original authors of this project chose to use pymongo's bson utils to handle JSON serialization and deserialization into various VARCHAR types in MySQL.

We have an unresolved security issue related to pymongo.

I've had questions in the past why are we using pymongo here, it suggests that sync-engine uses MongoDB while it does not. I think it's time to break that dependency because it just brings confusion, and pymongo will continue to receive security patches often as it contains C code which makes it prone to security bugs.

MongoDB's extended JSON syntax can serialize more types than stdlib's json module. The docs.

Pymongo's implementation sits in here https://github.com/mongodb/mongo-python-driver/blob/18328a909545ece6e1cd7e172e28271a59e367d5/bson/json_util.py. As you can see it handles many extra types. I analyzed all the call sites and data in those columns for \$\w references and concluded that we are only using "field": {"$date": <timestmp>} syntax in sync-engine.

Example

{"sync_disabled_by": null, "sync_start_time": {"$date": 1717004873429}, "original_start_time": {"$date": 1522779878389}, "sync_error": null, "sync_disabled_on": null, "sync_end_time": null, "sync_disabled_reason": null}

I've vendored the code and then stripped it to only include the parts that we are using.

Separately I think it makes sense to completely get rid of this $date syntax as I don't find it particularly readable.

I've already tested those changes first on my account in production and also on an entire cluster (nylas-e) and did not see any problems.

My intenral monologue about columns using JSON types

actionlog.extra_args

sync-engine/inbox/models/action_log.py

Line 67 in 64e70ec

extra_args = Column(JSON, nullable=True)

SELECT DISTINCT extra_args FROM (SELECT extra_args from actionlog a LIMIT 500000) As a;

extra_args
{"unread": false}
{"added_labels": [], "removed_labels": ["\Inbox"]}
{"destination": "INBOX.Archive"}
{"destination": "INBOX"}
{"added_labels": ["\Trash"], "removed_labels": ["\Inbox"]}
{"added_labels": ["\Inbox"], "removed_labels": []}
{"added_labels": [], "removed_labels": ["\Inbox", "\Trash"]}
{"added_labels": ["\Important", "\All"], "removed_labels": []}
{"added_labels": ["\Important"], "removed_labels": []}

account.sync_status

sync-engine/inbox/models/account.py

Line 182 in c064bf8

_sync_status = Column(MutableDict.as_mutable(JSON), default={}, nullable=True)

SELECT account._sync_status FROM account LIMIT 1;

_sync_status
{"sync_disabled_by": null, "sync_start_time": {"$date": 1717004873429}, "original_start_time": {"$date": 1522779878389}, "sync_error": null, "sync_disabled_on": null, "sync_end_time": null, "sync_disabled_reason": null}

SELECT DISTINCT REGEXP_SUBSTR(account._sync_status, "\\$\\w+") FROM account WHERE account._sync_status LIKE "%$%";

REGEXP_SUBSTR(account._sync_status, "\$\w+")
$date

imapuid.extra_flags

sync-engine/inbox/models/backends/imap.py

Line 125 in 1b7f90d

extra_flags = Column(LittleJSON, default=[], nullable=False)

SELECT DISTINCT extra_flags FROM (SELECT extra_flags from imapuid a LIMIT 50000) As a;

extra_flags
[]
["$NotJunk", "NotJunk"]
["$Forwarded", "Forwarded"]
["$Forwarded", "$NotJunk", "NotJunk"]
["$Forwarded"]
["$Phishing"]
["$NotJunk", "JunkRecorded", "NotJunk"]
["$Forwarded", "$NotJunk", "Forwarded", "JunkRecorded", "NotJunk"]
["$MailFlagBit0", "$NotJunk", "JunkRecorded", "NotJunk"]
["$MailFlagBit0", "$MailFlagBit2"]
["$MailFlagBit0"]
["$Forwarded", "$NotJunk", "Forwarded", "NotJunk"]
["$MailFlagBit0", "$MailFlagBit2", "$NotJunk", "JunkRecorded", "NotJunk"]
["$MailFlagBit1", "$NotJunk", "JunkRecorded", "NotJunk"]
["$MailFlagBit1"]
["$MailFlagBit0", "$MailFlagBit2", "$Phishing"]

imapuid.g_labels

sync-engine/inbox/models/backends/imap.py

Line 128 in 1b7f90d

g_labels = Column(JSON, default=list, nullable=True)

SELECT DISTINCT g_labels FROM (SELECT g_labels from imapuid a LIMIT 500000) As a;

g_labels
[]

message.from_addr message.sender_addr message.reply_to message.to_addr message.cc_addr message.bcc_adr message.in_reply_to

sync-engine/inbox/models/message.py

Lines 133 to 139 in 1b7f90d

    
           from_addr = Column(JSON, nullable=False, default=list) 
        
           sender_addr = Column(JSON, nullable=True) 
        
           reply_to = Column(JSON, nullable=True, default=list) 
        
           to_addr = Column(JSON, nullable=False, default=list) 
        
           cc_addr = Column(JSON, nullable=False, default=list) 
        
           bcc_addr = Column(JSON, nullable=False, default=list) 
        
           in_reply_to = Column(JSON, nullable=True)

SELECT from_addr, sender_addr, reply_to, to_addr, cc_addr, bcc_addr, in_reply_to FROM message LIMIT 100;

Looks like json encoded:
from_addr list[tuple[str, str]]
sender_addr list[tuple[str, str]]
reply_to list[tuple[str, str]]
to_addr list[tuple[str, str]]
cc_addr list[tuple[str, str]]
bcc_addr list[truple[str, str]]
in_reply_to str

metadata.value

sync-engine/inbox/models/metadata.py

Lines 50 to 51 in 05bc8c9

value = Column(JSON)

This table is empty

event.participants

sync-engine/inbox/models/event.py

Line 176 in 1b7f90d

participants = Column(MutableList.as_mutable(BigJSON), default=[], nullable=True)

list[{"status": enum, notes: str, guests: list[?], name: str, email: str}]

Details

squeaky-pl · 2024-06-03T09:08:36Z

-# Monkeypatch to not include tz_info in decoded JSON.
-# Kind of a ridiculous solution, but works.
-json_util.EPOCH_AWARE = EPOCH_NAIVE


Since this was indeed ridiculous I just inlined the monkeypatched code into the vendored module.

wojcikstefan · 2024-08-22T13:03:58Z

Thanks for doing this @squeaky-pl! Been long time coming.

squeaky-pl added 7 commits May 29, 2024 20:36

Vendor json_util from pymongo

dc1e5b6

Remove % formatting

05bc8c9

Remove monkeypatch

1e60b53

Remove types we don't need

9f0ddee

Drop bson dependency in json_util

6f000f4

Cleanup docstrings

5e8a711

Remove pymongo requirement

e6f4eff

squeaky-pl changed the title ~~json util~~ Remove pymongo dependency from this project Jun 3, 2024

squeaky-pl commented Jun 3, 2024

View reviewed changes

squeaky-pl mentioned this pull request Jun 3, 2024

Get rid of $date syntax in JSON documents in account._sync_status #775

Open

squeaky-pl requested review from drewler, mrhiggi-close and nsaje June 3, 2024 09:10

nsaje approved these changes Jun 5, 2024

View reviewed changes

squeaky-pl merged commit 2c3aad1 into master Jun 6, 2024

squeaky-pl mentioned this pull request Aug 22, 2024

Remove packages that were needed to build pymongo #863

Merged

wojcikstefan deleted the json-util branch August 22, 2024 13:03

Navigation Menu

Uh oh!

Remove pymongo dependency from this project - #771

Remove pymongo dependency from this project#771
squeaky-pl merged 7 commits into
masterfrom
json-util

squeaky-pl commented May 29, 2024 •

edited

Loading

Uh oh!

squeaky-pl Jun 3, 2024

Uh oh!

wojcikstefan commented Aug 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	from_addr = Column(JSON, nullable=False, default=list)
	sender_addr = Column(JSON, nullable=True)
	reply_to = Column(JSON, nullable=True, default=list)
	to_addr = Column(JSON, nullable=False, default=list)
	cc_addr = Column(JSON, nullable=False, default=list)
	bcc_addr = Column(JSON, nullable=False, default=list)
	in_reply_to = Column(JSON, nullable=True)

	value = Column(JSON)

Uh oh!

Conversation

squeaky-pl commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

squeaky-pl Jun 3, 2024

Choose a reason for hiding this comment

Uh oh!

wojcikstefan commented Aug 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

squeaky-pl commented May 29, 2024 •

edited

Loading